Run KubeVirt integration tests on GitHub Actions#287
Conversation
|
Skipping CI for Draft Pull Request. |
Reviewer's GuideRefactors integration tests and operator installation flow to support running KubeVirt-based integration tests in GitHub Actions, replacing ad‑hoc polling with kube runtime await_condition/timeouts, tightening test cleanup, and adding a dedicated Go module and CI workflow for virtctl/KubeVirt setup. Sequence diagram for updated operator reconcile installation flowsequenceDiagram
participant Reconcile as reconcile
participant InstallComponents as install_components
participant ReferenceValues as reference_values::adopt_approved_images
Reconcile->>InstallComponents: install_components(client, cluster)
alt [component installation fails]
InstallComponents-->>Reconcile: Err(e)
Reconcile-->>Reconcile: Action::requeue(Duration::from_secs(60))
else [component installation succeeds]
InstallComponents-->>Reconcile: Ok(())
Reconcile->>ReferenceValues: adopt_approved_images(client, cluster)
ReferenceValues-->>Reconcile: Ok(())
Reconcile-->>Reconcile: Action::await_change()
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
9ae710e to
ed83a9e
Compare
|
/ok-to-test |
98fb28f to
2be922c
Compare
2be922c to
f3e5378
Compare
f3e5378 to
47afcbe
Compare
|
first pass, 3 passes in a row & I'm happy |
b95e93a to
ffd95fe
Compare
|
3 passes, removing debug info |
ffd95fe to
7b3b8a6
Compare
|
Opening for review. To reviewers: This reuses some commits identified in #285, but I'm still opening it now because the feedback loop for making KubeVirt-on-KinD-GHA work is much quicker than Azure-on-OpenShift-CI. |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
tools/virtctl/go.modthego 1.25.0directive targets a Go version that is not yet generally available; consider aligning this with a currently supported Go release to avoidgo list/tooling failures when working in that module.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `tools/virtctl/go.mod` the `go 1.25.0` directive targets a Go version that is not yet generally available; consider aligning this with a currently supported Go release to avoid `go list`/tooling failures when working in that module.
## Individual Comments
### Comment 1
<location path="Makefile" line_range="21" />
<code_context>
YQ_VERSION ?= $(shell go list -m -f '{{.Version}}' github.com/mikefarah/yq/v4)
YQ ?= $(LOCALBIN)/yq-$(YQ_VERSION)
-KOPIUM_VERSION ?= $(shell grep kopium lib/Cargo.toml | sed -E 's/.*"(.*)"/\1/')
+KOPIUM_VERSION ?= $(shell cargo metadata --format-version 1 | jq -r '.resolve.nodes[] | select(.deps[]?.name == "kopium") | .deps[] | select(.name == "kopium") | .pkg | split("@")[1]')
KOPIUM ?= $(LOCALBIN)/kopium-$(KOPIUM_VERSION)
</code_context>
<issue_to_address>
**suggestion (performance):** Using `cargo metadata` + `jq` here introduces extra tooling requirements and may slow down repeated Make invocations.
This approach is nicer than grepping Cargo.toml, but it does mean:
- `jq` must be installed, and
- `cargo metadata` runs on every evaluation, which can noticeably slow CI or frequent `make` runs.
Consider either caching `KOPIUM_VERSION` via a helper script/Make target so it’s computed once and reused, or providing a simpler fallback (e.g., the previous approach) when `jq`/`cargo` aren’t available to keep `make` usable in minimal environments.
```suggestion
KOPIUM_VERSION := $(shell cargo metadata --format-version 1 2>/dev/null \
| jq -r '.resolve.nodes[] | select(.deps[]?.name == "kopium") | .deps[] | select(.name == "kopium") | .pkg | split("@")[1]' 2>/dev/null \
|| grep kopium lib/Cargo.toml | sed -E 's/.*"(.*)"/\1/')
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| YQ_VERSION ?= $(shell go list -m -f '{{.Version}}' github.com/mikefarah/yq/v4) | ||
| YQ ?= $(LOCALBIN)/yq-$(YQ_VERSION) | ||
| KOPIUM_VERSION ?= $(shell grep kopium lib/Cargo.toml | sed -E 's/.*"(.*)"/\1/') | ||
| KOPIUM_VERSION ?= $(shell cargo metadata --format-version 1 | jq -r '.resolve.nodes[] | select(.deps[]?.name == "kopium") | .deps[] | select(.name == "kopium") | .pkg | split("@")[1]') |
There was a problem hiding this comment.
suggestion (performance): Using cargo metadata + jq here introduces extra tooling requirements and may slow down repeated Make invocations.
This approach is nicer than grepping Cargo.toml, but it does mean:
jqmust be installed, andcargo metadataruns on every evaluation, which can noticeably slow CI or frequentmakeruns.
Consider either caching KOPIUM_VERSION via a helper script/Make target so it’s computed once and reused, or providing a simpler fallback (e.g., the previous approach) when jq/cargo aren’t available to keep make usable in minimal environments.
| KOPIUM_VERSION ?= $(shell cargo metadata --format-version 1 | jq -r '.resolve.nodes[] | select(.deps[]?.name == "kopium") | .deps[] | select(.name == "kopium") | .pkg | split("@")[1]') | |
| KOPIUM_VERSION := $(shell cargo metadata --format-version 1 2>/dev/null \ | |
| | jq -r '.resolve.nodes[] | select(.deps[]?.name == "kopium") | .deps[] | select(.name == "kopium") | .pkg | split("@")[1]' 2>/dev/null \ | |
| || grep kopium lib/Cargo.toml | sed -E 's/.*"(.*)"/\1/') |
for CI installation & dependabot autoupdates Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
This reverts commit cc6bf85. Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
or it may not be deleted, plus a spell fix Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
7b3b8a6 to
fed91ca
Compare
|
we got #285 merged and this is rebased (10 -> 5 commits) |
|
control plane didn't come up on the CI that this PR intends to replace /retest |
Also tests fine with 500m, which aligns better with how much compute we get on GHA. Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
avoiding maintenance and lack of parallelization on a CI host & scripting logic in openshift-ci Signed-off-by: Jakob Naucke <jnaucke@redhat.com> Assisted-by: Opus 4.6
fed91ca to
7979f4d
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alicefr, Jakob-Naucke The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
thanks for your review @alicefr. let's wait this run again and if it succeeds, I'll merge and raise the removal in openshift-release (-3kloc) |
de12968
into
trusted-execution-clusters:main
Test relied on moving PR code to another host and running KinD there, which has been superseded by running on GHA trusted-execution-clusters/operator#287. Keeping basic structure for trusted-execution-clusters because we still intend to utilize OpenShift on openshift#79393. Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
Summary by Sourcery
Refactor integration tests and supporting utilities to use Kubernetes await-based conditions and stricter cleanup, enhance operator installation error handling and logging, add configuration for KubeVirt VM CPU requirements, and introduce a GitHub Actions workflow and supporting Go module to run KubeVirt-based integration tests in CI.
Enhancements:
CI:
Documentation:
Tests: